-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Interpolate environment variables #1765
Interpolate environment variables #1765
Conversation
Will accept default values? e.g. |
def interpolate(string, mapping): | ||
try: | ||
return Template(string).substitute(defaultdict(lambda: "", mapping)) | ||
except ValueError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using safe_substitute()
would be closer to how os.path.expandvars()
worked previously (when keys are missing, leave the original value).
Otherwise this except
should also include KeyError
, which is raised when the value isn't in the environment (which I think should be a test case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I just realized that is what defaultdict is handling. So the result of a missing value would be empty string instead of leaving the original string. That should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the test file so that the "unset variable" case gets its own test, just so it's clear we do handle that case.
I like it. What do you think about moving all the interpolation stuff into a new module (since right now it's spread across I'm trying to figure out if we could extend this to support default values (not right now, but maybe at some later time). From what I can see, we'd have to duplicate a bunch of |
Yeah, makes sense to move it into a new module. We might be able to support default values by extending @elgalu No, it doesn't support default values. If we choose to support them in the future, the syntax should ideally be the same as the POSIX syntax, i.e. |
6901b61
to
c0da04c
Compare
c0da04c
to
bda8f82
Compare
01606af
to
51a34ce
Compare
Had a go at some docs - ping @moxiegirl |
I've split out interpolation logic into its own module, which necessitated some preliminary refactoring of |
@@ -361,6 +365,32 @@ Each of these is a single value, analogous to its | |||
read_only: true | |||
|
|||
|
|||
## Variable substitution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aanand a few tweaks:
Variable substitution
Your configuration options can contain environment variables. Compose uses the variable values from the shell environment in which docker-compose
is run. For
example, suppose the shell contains POSTGRES_VERSION=9.3
and you supply this configuration:
db:
image: "postgres:${POSTGRES_VERSION}"
When you run docker-compose up
with this configuration, Compose looks for the
POSTGRES_VERSION
environment variable in the shell and substitutes its value in. For this example, Compose resolves the image
to postgres:9.3
before running the configuration.
If an environment variable is not set, Compose substitutes with an empty
string. In the example above, if POSTGRES_VERSION
is not set, the value for
the image
option is postgres:
.
Both $VARIABLE
and ${VARIABLE}
syntax are supported. Extended shell-style
features, such as ${VARIABLE-default}
and ${VARIABLE/foo/bar}
, are not
supported.
If you need to put a literal dollar sign in a configuration value, use a double
dollar sign ($$
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
51a34ce
to
172fec2
Compare
One important question: when a variable is unset, should we substitute an empty string (as is currently implemented) or raise an error?
|
Personally, I like the flexibility of a warning (option 3). So, say I run a configuration across a set of hosts that I swap out or that aren't identical, I could have a value configured such that it is empty on one type of host but not another. Which could be useful... |
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
d396d2e
to
bb41e59
Compare
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
bb41e59
to
8b5bd94
Compare
Pushed some refactoring to make #1808 merge more easily. Also, we now show a warning when a variable is not set:
It could still be improved in that, if you reference the same variable multiple times, it'll show a warning for each one. |
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
511ca18
to
ee6ff29
Compare
Yay! This is ace 🎈 LGTM |
@rosskevin this has not been released yet (it's part of the coming 1.5 release), so if you want to test this feature, you'll have to build it yourself; you can find some instructions here https://github.com/docker/compose/blob/master/CONTRIBUTING.md If you're question is "how to run compose" (in general); docs are here https://docs.docker.com/compose/ |
Can't wait for this... I'm looking for passing environment variables for DEV and PROD instances. |
@marcellodesales a release candidate for 1.5 is available for testing, see https://github.com/docker/compose/releases/tag/1.5.0rc1. It should be used with care of course, release candidates can still contain bugs |
@thaJeztah Sounds good... I will be playing with it this week and I may report anything weird... |
I think this caused a regression in behaviour. My docker-compose.yml looks like this: postgres:
container_name: mailserverdocker_postgres
build: postgres
environment:
POSTGRES_PASSWORD:
PGDATA: /var/lib/postgresql/data/pgdata
volumes:
# This is not on the host but on the docker-machine. Cos VMHGFS permissions
# are fail. FULL OF FAIL AND PERMISSIONS ERRORS
- /docker-volumes/run/pg-data:/var/lib/postgresql/data Previously this would use the
I can fix it by changing to Should I open another bug for this issue? (Maybe just a documentation change and a note in the changelog about this being a (small) breaking change) |
Hi @ashb , can you open a new issue with this info in it for me please? I can't triage and assign the issue properly if it's in a PR comment. Thank you 👍 |
+1 to fetching default variables for environment variables from a |
^ I actually take that back. Defining a |
What I'd like to do is use the contents of an env-file, or environment block in variable subsitution. I.e., I have a VERSION file which contains
There currently does not seem to be a way to do this and still keep using a normal Any idea how to tackle this would be much appreciated. |
Hi @aanand! I want to know if you implemented default values? |
Can we also use the default values when env vars are not defined? version: $VERSION:0.1.0 Not defining |
Default values have not been implemented. There's a WIP implementation in #3108. |
Since DockerCompose uses How do I force Docker-Compose to use a custom named environmental file? For example I believe I cannot use |
Uses
string.Template
. No docs yet, but here's what works:Closes #1377.